-
Notifications
You must be signed in to change notification settings - Fork 60
fix(piv): Fix YubiKeySignatureGenerator.DigestData regression in Sample App #398
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
DennisDyallo
commented
Jan 22, 2026
- Fixed DigestData to use hash digest size instead of key size for RSA keys
- Reuses MessageDigestOperations.ComputeMessageDigest for hashing
- For RSA: returns raw digest (PadRsa handles signature padding)
- For ECC: pads digest to key size with leading zeros if needed
- Added unit tests for digest computation logic
- Updated devcontainer to include .NET 8.0 and 10.0
- Fixed DigestData to use hash digest size instead of key size for RSA keys - Reuses MessageDigestOperations.ComputeMessageDigest for hashing - For RSA: returns raw digest (PadRsa handles signature padding) - For ECC: pads digest to key size with leading zeros if needed - Added unit tests for digest computation logic - Updated devcontainer to include .NET 8.0 and 10.0
Test Results: Windows 2 files 2 suites 15s ⏱️ Results for commit fb4d208. ♻️ This comment has been updated with latest results. |
Test Results: Ubuntu 2 files 2 suites 47s ⏱️ Results for commit fb4d208. ♻️ This comment has been updated with latest results. |
Test Results: MacOS 4 files 4 suites 30s ⏱️ Results for commit fb4d208. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a regression in the YubiKeySignatureGenerator.DigestData method where the key size was incorrectly used instead of the hash digest size for RSA keys, causing ArgumentException when trying to copy more bytes than available in the hash result.
Changes:
- Fixed DigestData to return raw digest for RSA keys (signature padding is handled by PadRsa)
- Changed ECC digest handling to pad with leading zeros when digest is smaller than key size
- Added comprehensive unit tests to verify the fix and prevent future regressions
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| YubiKeySignatureGenerator.cs | Fixed DigestData to use hash digest size instead of key size for RSA, reuses MessageDigestOperations.ComputeMessageDigest, and properly pads ECC digests |
| YubiKeySignatureGeneratorTests.cs | Added unit tests verifying correct digest computation for RSA and ECC keys, including tests demonstrating the original bug |
| devcontainer.json | Added .NET 8.0 and 10.0 to the development container configuration |